feat(InstanceSort): auto-align nulls_first for index utilization, warn on mismatch#2613
feat(InstanceSort): auto-align nulls_first for index utilization, warn on mismatch#2613haakonvt wants to merge 8 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces automatic nulls_first derivation for InstanceSort to optimize PostgreSQL index utilization and adds UserWarning alerts for non-aligned sort configurations across query, sync, and subscribe operations. Feedback indicates that always setting a boolean for nulls_first causes a regression in instances.search(), which explicitly disallows the parameter and will now raise a ValueError.
haakonvt
changed the title
…ly_postgres_defaults_or_maybe_warn
…nc) class hierarchy
63a94ff to
12103ea
Compare
haakonvt
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2613 +/- ##
==========================================
+ Coverage 93.02% 93.06% +0.04%
==========================================
Files 486 486
Lines 49462 49634 +172
==========================================
+ Hits 46010 46192 +182
+ Misses 3452 3442 -10
🚀 New features to boost your workflow:
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces automatic PostgreSQL index alignment for instance sorting in data modeling. It enhances the InstanceSort class to automatically set or validate the nulls_first parameter based on the sort direction, ensuring optimal database performance. Additionally, it implements a traversal mechanism within the Query class to apply these defaults across all nested sort configurations. I have no feedback to provide.
There was a problem hiding this comment.
Code Review
This pull request introduces automatic PostgreSQL index alignment for InstanceSort by resolving nulls_first based on the sort direction and providing warnings for misaligned configurations. It includes updates to the instances API to prepare sorts before execution and adds corresponding unit tests. Feedback was provided regarding the mutation of InstanceSort objects in the _apply_postgres_defaults_or_maybe_warn method, which may cause compatibility issues with Elasticsearch-based search operations if the sort object is reused.
2 participants
Summary
InstanceSortnow validatesdirectionat construction time (case-insensitive, raisesValueErroron bad input)nulls_first=None(default) is resolved lazily at PostgreSQL-backed call sites (list,query,sync,subscribe) via the new_apply_postgres_defaults_or_maybe_warn()method, keeping the wire format clean for Elasticsearch-backed endpoints (search,aggregate)UserWarningis emitted when an explicitnulls_firstvalue conflicts with index alignment for the given directionis_index_alignedproperty for inspecting alignment without side effectsQuery,QuerySync) gain_iter_sorts()traversal and_prepare_sorts()to apply defaults across all nested sorts in one callNote 2 reviewer:
Validation and auto-conversion from
NonetoTrue/Falsewas initially put onInstanceSortitself, but there is an existingNone-check oninstances.search(...)raisingValueErrorif set to non-null, so that's why this grew a bit out of proportions and had to be added to all "API call sites hitting postgres"